fix(windows): Invoke-Checked rejected the empty argument list every no-arg test uses (#512) - #583
Conversation
…o-arg test uses (#512) `windows-msvc-cpu` has failed on every pull request, ~21 minutes in, with zero compile diagnostics: Cannot bind argument to parameter 'Arguments' because it is an empty array. `Invoke-Checked` declared `Arguments` as `[Parameter(Mandatory)][string[]]`. PowerShell's `Mandatory` validation treats an empty collection as "not supplied", so every call that runs a test executable taking no arguments died at parameter binding before the process was ever started. There are six such production call sites, not four -- the two forced-CPU-tier invocations (`VT_CPU_MATMUL_TIER=portable` / `avx2`) are on the same path and would have failed next. The fix is `[AllowEmptyCollection()]` alongside `Mandatory`, not a `= @()` default. The existing intent is that a caller must state its argument list; `AllowEmptyCollection` keeps the omission an error while permitting an explicitly empty list, whereas a default would silently accept a call that forgot the parameter entirely. The contract step ran green all along because the suite never executed an empty-argument invocation, which is why a 21-minute build step caught what a seconds-long contract step should have. `Invoke-CheckedContractTests` closes that: it injects a recording runner -- mirroring the `DumpbinRunner` and unsupported-tier-probe seams already in this file -- and asserts the empty and non-empty argument lists are forwarded verbatim and that a nonzero status still throws on both. `Invoke-Checked` gains the matching optional `-Runner` seam. RED-first, run under pwsh 7.6.4: the new contract test fails with the exact CI message above before `[AllowEmptyCollection()]` is applied, and passes after. Five mutations of the claimed guarantees are each caught by their intended assertion (drop `AllowEmptyCollection`; swallow the argument list; never throw on nonzero; forward the wrong program; truncate the arguments), and the real `& $Program @Arguments` path -- not just the fake runner -- was exercised against `/bin/true`, `/bin/echo a b`, and `/bin/false`. The pre-existing unsupported-tier contract assertion is untouched and proven still non-vacuous: making the probe send an empty, a wrong, or a two-element argument list each still trips "did not receive one exact filter argument". That empty-args mutation also shows the fake-runner scriptblock parameters need no `AllowEmptyCollection` of their own -- being non-mandatory, they bind `@()` and fail on the assertion rather than on binding. This is a distinct defect from #514, the POSIX `setenv`/`unsetenv` C3861 error that fails `windows-msvc-vulkan`; that one is fixed on its own branch and `windows-msvc-vulkan` stays red here until it lands. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
CI result — read from the completed job, not predicted#512 is closed.
The contract step passing on the real Windows runner is the direct proof: the Baseline subtraction
The empty-array message is gone. The script ran past all six empty-argument
What #512 was masking — filed as #584
That is a defect in the test binary, not in this PowerShell change — this PR This PR is still the correct and complete fix for #512: it converts a failure |
Brings in #583, which repairs the Invoke-Checked empty-argument defect that was red on every open PR including this one, plus the Nemotron and intake records. No overlap with this branch, which touches the DSpark spec, the benchmark record, STATUS, BENCHMARKS and two standalone harnesses. Worth noting for this row specifically: BENCH-ASSERT-CLOCK-STATE (51ec6be) records that the SM clock moves 12.8% between boots. That is the same class of effect this branch records as "absolute numbers move up to 5% between sessions for the same binary", and it independently reinforces why only WITHIN-session, interleaved ratios are quotable here. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…contract (#512) (#589) FOLLOWING_AGENTS_PROTOCOL Pins two properties #583 left unguarded. A fresh review of that PR found four mutations surviving its contract suite -- most importantly setting $exitCode = 0 in the real-process branch, and dropping the @ splat. Either means a future edit passes the contract suite SILENTLY while the Windows gate reports success for tests that failed or never ran. That is the failure class this repo keeps paying for, and it is the branch #583 restructured and the branch that surfaced #584. All four now RED: real-branch $exitCode = 0; dropped @ splat; dropped Mandatory; added AllowNull. The seven already caught stay caught, including the one that distinguishes "forwarded an empty array" from "forwarded nothing" -- @($null) has Count 1 and would otherwise sneak past. Two findings worth keeping. A naive omission test would HANG A DEVELOPER'S TERMINAL: an omitted mandatory parameter PROMPTS under an interactive console host, blocking on "Arguments[0]:" until killed at 25 s under a pty, and only reaches the binding error in CI where stdin is not a tty -- a test meaning one thing in CI and another on a desk. It is asserted in an API runspace instead, whose host cannot prompt, and the function under test is rebuilt from the live definition's own source text so it tracks edits to the real parameter block. And the @-splat mutation is nearly an equivalent mutant, disclosed rather than oversold: for a NATIVE executable the splatted and unsplatted forms give identical argv at 0, 1 and 3 elements under all three $PSNativeCommandArgumentPassing modes including Windows. It is only observable when the program is a PowerShell script, where an empty list otherwise arrives as one array argument instead of zero -- which is exactly #512's contract. The real-process arm drives the PowerShell host executing the script, resolved from (Get-Process -Id $PID).Path: the one executable guaranteed to exist wherever the script can run, so the IDENTICAL arm runs on Windows runners and POSIX boxes with no platform branch. An arm that silently no-ops on one platform would be its own version of the bug being fixed. Operator gate: contract suite rc=0, and the previously-surviving real-branch mutation re-applied by hand fails with "real-process nonzero exit status was accepted", tree restored byte-for-byte. CI run conclusion success, 0 real failures. Still not green and not claimed to be: windows-msvc-cpu needs #584, vulkan needs #514, and #585 remains filed. Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [Claude Code]
…the C++ compile (#774) (#795) Closes #774. `check-windows-portability.py:1710` asserted the MSVC warning policy with a SUBSTRING test: `if not all(token in warnings for token in ("/W4", "/WX"))`. `"/WX" in "/WX-"` is True, and `/WX-` is MSVC's spelling for DISABLE warnings-as-errors -- so the gate was blind to its own inversion. Measured against PR #640 commit 74ba382, which shipped exactly that: `/WX-` on the CXX arm, with the only bare `/WX` left on $<COMPILE_LANGUAGE:OBJCXX> -- Objective-C++, the Metal backend, which never compiles under MSVC. The checker passed it. Two further blindnesses fell out of the same `in`, both found while scoping and neither in the issue: `/W44996` answers for `/W4`, and CMakeLists.txt:30's `#` comment contains the literal `/W4 /WX`, satisfying the entire policy on its own -- deleting every real flag would still have passed. The repair is a token-boundary match evaluated over flags reduced to what can reach an MSVC C/C++ TU (comments stripped, genexes naming only non-C/C++ languages blanked in place), plus refusal of the negating spellings `/WX-`, `/W0`, `/w`. Sibling evasions argued OUT in the spec rather than silently swept in: COMPILE_WARNING_AS_ERROR OFF (CMake only uses it to decide whether IT adds a flag; it does not remove a literal /WX from target_compile_options) and blanket /wd#### (narrows what /W4 reports, does not invert it; "how many is too many" is an undecided threshold). `/W0` and `/w` are IN because they are the disable spellings of /W4 itself. RED before, same test file against base and head checkers: 6 failed, 1 passed E AssertionError: 0 == 0 : Windows portability contract OK That message is the finding -- the gate reporting "contract OK" on a tree whose C++ arm says /WX-. The single base pass is the inverse pin, so the fix is not merely stricter about everything. GREEN after: 7 passed, 9 subtests. Also carries a repair it did not cause: tests/scripts/test_check_windows_ portability.py has been RED on main since e8a9e74 (#680's stale mutation anchor -- the mutation targets the first `$calls.Add(` in the file, which since #512/#583 lives in a different function). check-pr-size's evidence contract runs the whole recognized module and requires it green at HEAD, so nothing could land in that file until this was fixed. The mutation is now anchored to the governed occurrence with uniqueness asserted; #680 stays open for its other half. Full tests/scripts: 8 failed / 1368 passed, all eight reproduced BY NAME on a pristine origin/main worktree. CI: merged with checks still queued -- the runner pool has been saturated for hours and no job started on this head. Every gate runnable locally is green, including check-pr-size's own re-execution of the base-red/head-green evidence in an isolated worktree, which is the authoritative form of that proof.
Closes #512.
windows-msvc-cpufailed on every PR with:Invoke-Checked's$Argumentsis[Parameter(Mandatory)]and six call sites pass@()— PowerShell treats an empty collection as "not supplied".Fix
[AllowEmptyCollection()]alongsideMandatory, not a= @()default. The intent is that a caller must state its argument list, so omission should stay an error while an explicitly empty list is permitted. A fresh review proved both halves rather than accepting the argument: omission still raisesmissing mandatory parameters: Arguments, while the rejected= @()form silently accepts it.Also adds
Invoke-CheckedContractTests. The pre-existing contract suite passed green today — which proves it never exercised an empty-argument call, or it would have caught this. A 21-minute Windows build step was catching what a seconds-long local step should have.pwshruns this path on Linux, so it does now.Success criterion — narrow, and NOT "the job is green"
Closing #512 unmasked a hard crash.
test_openai_api_server.exedies with-1073740791=0xC0000409(STATUS_STACK_BUFFER_OVERRUN) before doctest prints anything — zeroStatus:lines, zeroassertions:lines, just the version banner. That binary had never once executed on Windows, so the crash was latent for as long as #512 was. Filed as #584.What this PR claims, verified from completed jobs:
windows-msvc-cpuempty arraySo: the empty-array error is gone and the script now runs past all six sites on both real Windows runners.
windows-msvc-cpucannot go green until #584 lands, andwindows-msvc-vulkanstays red on #514.🤖 Generated with Claude Code